Skip to content

Conversation

@mernst
Copy link
Contributor

@mernst mernst commented Jan 12, 2026

Merge after #27.

@coderabbitai
Copy link

coderabbitai bot commented Jan 12, 2026

Warning

Rate limit exceeded

@mernst has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 21 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3c8efb8 and 0c3cd37.

📒 Files selected for processing (1)
  • git-clone-or-pull
📝 Walkthrough

Walkthrough

Adds a new shell script git-clone-or-pull that clones a GitHub repository or pulls updates if the destination exists, with default shallow-clone args, usage validation, destination-parent creation, and 180s timeouts with single-retry behavior on failures. Updates README.md to document git-clone-or-pull. Refactors git-clone-related to replace brace/pipeline-based retry logic with explicit if-based retry flow. Updates the test harness to pass an optional DEBUG flag into git-clone-related invocations.

Possibly related PRs

  • plume-lib/git-scripts PR 26: Modifies git-clone-related clone-retry logic, introducing similar sleep/retry and adjusted retry/error messaging.
  • plume-lib/git-scripts PR 28: Edits git-clone-related and its test driver, affecting the same script and test invocation changes updated here.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 32-34: The defaults set with set -- -q --single-branch --depth 1
are never applied because the git clone invocations don't pass "$@", and some
clones use the --depth=1 form which conflicts with the spaced default; update
the git clone calls (both the branch that handles the initial clone and the
fallback clone branch referenced around the other clone block) to include "$@"
after git clone and change any --depth=1 occurrences to the spaced form --depth
1 so the default GIT_CLONE_ARGS ($@) are honored.
- Around line 18-21: The assignment after the first shift is using the wrong
positional parameter: after executing shift, $1 now holds the repository name
but the code sets REPO_NAME=$2; change the assignment to REPO_NAME=$1 (and then
shift again if you intend to consume the repository name) so ORG, REPO_NAME, and
any subsequent DESTINATION_PARENT come from the correct positional parameters.
- Around line 5-7: The usage doc has inconsistent parameter names (it uses REPO
on line with "Usage" and REPO_NAME in the descriptive lines); pick one canonical
name (e.g., REPO_NAME) and update the "Usage: git-clone-or-pull ORG REPO
[DESTINATION_PARENT] [GIT_CLONE_ARGS]" line to use that same identifier, and
also update any other occurrences so all references to the repository parameter
are consistently named (REPO or REPO_NAME) across the file.
- Around line 25-28: The if-block incorrectly exits after successfully creating
DESTINATION_PARENT; change the logic so you only exit on failure: when [ ! -d
"${DESTINATION_PARENT}" ] then attempt mkdir -p "${DESTINATION_PARENT}" and if
that command fails (check its exit status or use a conditional/multi-command
like mkdir -p ... || exit 1) then exit with status 1; remove the unconditional
exit 1 that runs after a successful mkdir so the script continues on success.
- Line 22: The script calls an undefined function beginswith in the conditional
if [ "$#" -ne 0 ] && ! beginswith "-" "$1"; then — add a beginswith function
definition before that usage that accepts a prefix and a string and returns
success (0) when the string starts with the prefix and failure (non‑0)
otherwise; ensure the function handles empty inputs and is POSIX‑sh compatible
(use parameter expansion or case/pattern matching) and export no side effects so
the existing conditional using beginswith "-" "$1" works as intended.
- Around line 44-47: The retry branch of the git clone block can leave a
corrupted partial clone in "${DESTINATION}" so the second attempt will fail;
update the clone-retry logic around the existing if/then that uses timeout,
REPO_URL and DESTINATION to detect and remove any existing DESTINATION (rm -rf
"${DESTINATION}") before running the second timeout git clone, and also fix the
missing closing quote in the second git clone argument so the retry runs against
the correct path.
- Around line 43-47: Remove the redundant mkdir line and fix the unclosed double
quotes in the git clone commands: delete the unnecessary mkdir -p
"${DESTINATION_PARENT} (since directory creation is handled earlier), and ensure
both git clone invocations properly quote DESTINATION and REPO_URL (e.g., close
the double quotes around "${DESTINATION}" and "${REPO_URL}"), and keep the
existing timeout/clone retry logic using the symbols timeout, git clone,
REPO_URL, and DESTINATION.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16e5815 and 98dbe44.

📒 Files selected for processing (3)
  • README.md
  • git-clone-or-pull
  • git-clone-related
🧰 Additional context used
🪛 GitHub Actions: Check style
git-clone-or-pull

[error] 48-48: shfmt formatting error in file 'git-clone-or-pull': 'fi' can only be used to end an if.

🪛 GitHub Actions: Run Small test
git-clone-related

[error] 1-1: Test failed: expected repository URL 'https://github.com/mernst/git-clone-related-test-repo2.git' but got 'https://github.com/plume-lib/git-clone-related-test-repo2.git'. This caused the git-clone-related-test to fail during 'make test1'.

🔇 Additional comments (3)
git-clone-related (2)

158-167: Retry logic looks correct.

The retry sequence properly:

  1. Attempts clone with 180s timeout
  2. On failure, removes partial clone directory
  3. Sleeps 180s before retry
  4. Attempts second clone with same timeout
  5. Exits with status 2 on persistent failure

169-170: The fork detection issue is not in these lines.

Lines 169-170 only output the final commit hash after cloning. The actual fork detection occurs earlier at line 141 when git-find-fork is called. If the test expects the mernst organization fork but receives plume-lib, investigate the git-find-fork script logic or how CI_ORGANIZATION is determined by the ci-info script, not these output lines.

Likely an incorrect or invalid review comment.

README.md (1)

40-43: Documentation addition looks good.

The new section follows the established pattern and correctly links to the script for detailed documentation.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @tests/git-clone-related-test/test-git-clone-related.sh:
- Around line 18-20: The DEBUG variable is being unconditionally enabled because
DEBUG=--debug overwrites DEBUG= on the next line, which prevents the -z "$DEBUG"
cleanup branch from running and always emits debug output; fix this by making
DEBUG off by default (remove the unconditional DEBUG=--debug line or make that
assignment conditional) or swap the assignments so the script sets DEBUG=--debug
only when explicitly requested, ensuring the cleanup block that checks -z
"$DEBUG" can run in CI.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98dbe44 and 8af23d4.

📒 Files selected for processing (1)
  • tests/git-clone-related-test/test-git-clone-related.sh
🧰 Additional context used
🪛 GitHub Actions: Run Small test
tests/git-clone-related-test/test-git-clone-related.sh

[error] 1-1: Test failed: expected repo https://github.com/mernst/git-clone-related-test-repo2.git, got: https://github.com/plume-lib/git-clone-related-test-repo2.git. Command: make test (test-git-clone-related.sh) exited with code 2.

🔇 Additional comments (3)
tests/git-clone-related-test/test-git-clone-related.sh (3)

42-43: Shellcheck directive and invocation look correct.

The SC2086 disable is appropriate here since $DEBUG and $ARGS are intentionally subject to word splitting. The command structure correctly passes the debug flag and arguments to the script.


29-31: Variable renaming and cleanup are consistent.

The rename from goaldir to resultdir better reflects the variable's purpose (the result of cloning, not the goal). The cleanup logic correctly references both temporary directories.


53-61: Test repository setup issue, not a code logic problem.

The test expects mernst/git-clone-related-test-repo2 to be cloned, but git-find-fork is correctly falling back to plume-lib/git-clone-related-test-repo2 because the mernst fork either doesn't exist on GitHub or isn't accessible. The git-find-fork script works as designed: it checks for the fork's existence with wget and falls back to the upstream organization if the fork is unavailable. The git-clone-related script implements this fallback behavior correctly.

The test failure indicates that mernst/git-clone-related-test-repo2 is missing from the test repository setup or the organization no longer owns it. Ensure the required fork repositories exist on GitHub with the expected branches (branch-downstream-all, etc.) before running the tests.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 27-30: The current check that creates DESTINATION_PARENT then
immediately calls exit 1 stops the script erroneously; update the block around
the DESTINATION_PARENT check so it only exits on mkdir failure (i.e., run mkdir
-p "${DESTINATION_PARENT}" and if that fails then exit 1), otherwise allow the
script to continue; modify the if block containing DESTINATION_PARENT and the
mkdir -p invocation accordingly so normal flow proceeds after successful
directory creation.
- Around line 24-26: The script calls beginswith in the DESTINATION_PARENT
handling (if [ "$#" -ne 0 ] && ! beginswith "-" "$1"; then) but beginswith is
not defined here; add a beginswith function (or source the file that defines it)
before this usage that returns success if the first arg starts with the second
arg, or inline the equivalent check, so the conditional no longer triggers a
"command not found" error.
- Around line 46-48: The clone commands currently hardcode "--depth=1" and omit
the user-provided clone args, and the retry invocation also drops "-q"; update
both git clone invocations to use the collected user args (e.g. "$@" or the
GIT_CLONE_ARGS array/variable used earlier) instead of hardcoded options so the
defaults from line 35 are honored, and ensure the retry uses the exact same args
(including -q) as the first attempt.
- Around line 5-7: Align the terminology between the usage line and description:
change the usage line from "git-clone-or-pull ORG REPO [DESTINATION_PARENT]
[GIT_CLONE_ARGS]" to use "REPO_NAME" (i.e., "git-clone-or-pull ORG REPO_NAME
[DESTINATION_PARENT] [GIT_CLONE_ARGS]") so it matches the description, or
alternatively update the description to use "REPO"—pick one consistent
identifier and update both occurrences accordingly.
- Around line 42-50: When the initial git clone into "${DESTINATION}" fails the
script must remove any partially created directory before retrying; update the
failure branch of the clone (the block that does "if ! timeout 180 git clone -q
--depth=1 \"${REPO_URL}\" \"${DESTINATION}\"; then") to run rm -rf
"${DESTINATION}" (or at least remove contents) after the sleep and before the
second "timeout 180 git clone --depth=1 ..." so the retry runs against a clean
path; ensure the same cleanup is applied wherever a clone retry is attempted
(reference the clone commands using "${DESTINATION}" and "${REPO_URL}").
- Around line 22-23: The script incorrectly assigns REPO_NAME=$2 after calling
shift, which moves ORG out so the repository argument becomes $1; change the
assignment to use the correct positional parameter by either setting REPO_NAME
before the shift (assign REPO_NAME from $2 while $1 is ORG) or, if keeping the
shift, set REPO_NAME=$1 afterwards; update the line referencing REPO_NAME
accordingly (symbols: REPO_NAME, shift, ORG, REPO).
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8af23d4 and a308bf6.

📒 Files selected for processing (4)
  • README.md
  • git-clone-or-pull
  • git-clone-related
  • git-find-fork
🔇 Additional comments (5)
git-find-fork (1)

13-15: LGTM!

Good refactor to use SCRIPT_NAME for consistent error messaging across the related scripts. The -- in basename -- "$0" correctly handles edge cases where the script name might start with a hyphen.

git-clone-related (3)

59-64: LGTM!

Clean introduction of SCRIPT_NAME for consistent messaging. The usage message correctly references the script name dynamically.


160-169: LGTM!

The retry logic with 180-second timeout and 180-second sleep between attempts is appropriate for large repositories like the JDK. The cleanup of the failed destination (rm -rf "${DESTINATION}") before retry is correct.


171-172: LGTM!

Good use of SCRIPT_NAME in the final status message for consistency.

README.md (1)

40-44: LGTM!

Documentation for the new script is properly added in alphabetical order with consistent formatting.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 46-47: The git pull retry block silently swallows failures via "||
true" and mixes quiet flags between attempts; remove the "|| true" so failures
propagate (or replace it with an explicit warning/log and non-zero exit if you
prefer), make both pull attempts use the same "-q" flag for consistent output,
and ensure the retry logic logs a clear warning including DESTINATION and the
git error if both pulls fail so callers don't assume a successful update; update
the block around the (cd "${DESTINATION}" && (timeout 180 git pull -q || (sleep
1m && (timeout 180 git pull || true)))) expression accordingly.
- Around line 50-53: The retry can hit a partially-created DESTINATION from the
failed git clone; after the first clone fails, before the second timeout git
clone call, detect and remove any partial DESTINATION (safely: ensure
DESTINATION is non-empty and then rm -rf -- "${DESTINATION}") so the retry
performs a fresh clone instead of entering the pull/partial-directory path.
Ensure the cleanup runs only on failure of the first clone and before the second
git clone attempt.
- Around line 46-54: The script uses the external timeout utility around git
commands (e.g., in the pull branch where (timeout 180 git pull -q ...) and in
the clone branch where timeout 180 git clone ...) which will fail on systems
without GNU coreutils (macOS). Either add a small POSIX fallback that implements
a timeout using background jobs and kill (detect absence of timeout and use
fallback), or explicitly document the dependency by adding a README note and a
preflight check in the script that tests for timeout and emits a clear error
advising to install coreutils (or use gtimeout) before proceeding; update the
sections referencing timeout (git pull/clone blocks) and the script header to
perform the detection and choose fallback or fail fast with guidance.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a308bf6 and 04d70c8.

📒 Files selected for processing (2)
  • git-clone-or-pull
  • tests/git-clone-related-test/test-git-clone-related.sh
🔇 Additional comments (6)
tests/git-clone-related-test/test-git-clone-related.sh (2)

18-19: LGTM!

Simple and clear debug toggle pattern. The commented alternative makes it easy to enable debugging when needed.


43-44: LGTM!

The shellcheck directive correctly documents that both $DEBUG and $ARGS are intentionally unquoted, and the invocation order is logical.

git-clone-or-pull (4)

1-18: LGTM!

Clear documentation and proper argument validation with helpful usage message.


20-36: LGTM!

The argument parsing logic correctly handles the ambiguity between DESTINATION_PARENT and GIT_CLONE_ARGS using the dash-prefix heuristic.

Minor: The [ ! -d ... ] check on line 31 is redundant since mkdir -p is idempotent, but it's harmless.


37-44: LGTM!

Good default arguments for shallow cloning, and clean URL/path construction.


56-56: LGTM on output format.

The output is informative. Note that if the clone/pull operations failed (per previous comments), this line may fail or report stale data.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 51-55: The cleanup rm command is deleting the wrong path causing a
no-op; when the initial timeout git clone fails it created "${DESTINATION}"
(which is the full path including "${REPO_NAME}"), but the script calls rm -rf
"${DESTINATION:?}/${REPO_NAME}" (expanding to
DESTINATION_PARENT/REPO_NAME/REPO_NAME). Replace that rm -rf invocation with one
that removes "${DESTINATION}" instead so the failed clone directory is actually
removed before the retry of timeout 180 git clone "$@" "${REPO_URL}"
"${DESTINATION}".
- Around line 54-56: The retry clone block that runs `timeout 180 git clone "$@"
"${REPO_URL}" "${DESTINATION}"` lacks error handling: after a failed retry the
script proceeds to `cd "${DESTINATION}"` and will break if the directory doesn't
exist; modify the block to check the exit status of that second `git clone` and,
if non-zero, write an error message and exit with status 2 (matching the
`git-clone-related` behavior) so the script stops cleanly when the retry fails.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04d70c8 and d2b37f3.

📒 Files selected for processing (2)
  • git-clone-or-pull
  • git-clone-related
🔇 Additional comments (4)
git-clone-related (1)

168-177: LGTM! Clean refactor of retry logic.

The explicit if-based control flow is easier to follow than the previous brace-enabled sequence. Dropping the -q flag on retry is a nice touch for debugging failed clones.

git-clone-or-pull (3)

1-19: LGTM!

Clear documentation and proper usage validation.


21-41: LGTM!

The argument parsing logic correctly handles optional DESTINATION_PARENT by checking if the next argument starts with -.


58-58: LGTM!

Consistent output format with git-clone-related.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 26-37: The script redundantly calls mkdir -p for
DESTINATION_PARENT twice: once when handling the first argument (the block that
sets DESTINATION_PARENT and creates it if missing) and again in the later else
branch; remove the redundant mkdir -p in the else branch (or change that branch
to only create DESTINATION_PARENT if it does not exist) so DESTINATION_PARENT is
created only once; refer to the DESTINATION_PARENT variable and the two mkdir -p
occurrences to locate and update the duplicated creation logic.
- Line 48: The git pull command in the subshell that runs in DESTINATION uses a
1 minute sleep and ends with "|| true", causing inconsistent retry delays with
the clone path and silencing failures; change the retry delay to 180s to match
the clone logic and remove the trailing "|| true" so failures are not
swallowed—on pull failure run the same diagnostic output used in the clone flow
(e.g., run "git branch" or equivalent diagnostic command) so users see failure
details.
- Line 61: The script currently masks pull failures with "|| true" and then
blindly runs echo "${SCRIPT_NAME}: ${REPO_NAME} is at $(cd "${DESTINATION}" &&
git rev-parse HEAD)"; remove the silent swallow or add explicit checks: after
the pull attempts (the commands using "git pull" that currently use "|| true")
test their exit status and exit with an error if they failed, or before
reporting HEAD run cd "${DESTINATION}" && git rev-parse --verify HEAD >/dev/null
2>&1 and if that fails log an error via "${SCRIPT_NAME}" and exit non‑zero;
ensure the final echo only runs when cd "${DESTINATION}" && git rev-parse HEAD
succeeds so it never reports stale or missing HEAD.

In @git-clone-related:
- Around line 178-187: The retry block around the timeout git clone is
inconsistent: change the echo that currently says "after timeout" to a neutral
message like "after failure" (it can fail for reasons other than timeout) and
make the second git clone use the same flags as the first (add -q to the retry)
or alternatively remove -q from the error message so they match; ensure the
final error echo that references SCRIPT_NAME, REPO_BRANCH, REPO_URL and
DESTINATION matches the actual git clone command used (keep flags consistent
between the two git clone invocations and the error text).

In @tests/git-clone-related-test/test-git-clone-related.sh:
- Around line 44-47: The echo on line showing the planned command omits the
$DEBUG variable, so update the first echo to mirror the actual invocation by
including $DEBUG in the printed command (i.e., change the echo text to include
$DEBUG just as the real execution line uses "${GIT_SCRIPTS}"/git-clone-related
$DEBUG $ARGS "$resultdir"); ensure the variable placement and spacing match the
real command so the logged command and executed command are consistent.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2b37f3 and 2189306.

📒 Files selected for processing (3)
  • git-clone-or-pull
  • git-clone-related
  • tests/git-clone-related-test/test-git-clone-related.sh
🔇 Additional comments (3)
tests/git-clone-related-test/test-git-clone-related.sh (1)

18-19: LGTM! Clean approach for optional debug mode.

The DEBUG variable pattern allows easy toggling for debugging without modifying the script invocation.

git-clone-or-pull (2)

1-19: LGTM! Clear documentation and argument validation.

The usage documentation is clear and the early exit on insufficient arguments is appropriate.


51-58: Clone retry logic is correct and consistent with git-clone-related.

The timeout and retry pattern matches the updated git-clone-related script. Proper cleanup with rm -rf before retry.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 47-48: The branch that handles existing DESTINATION only checks
for a directory (if test -d "${DESTINATION}") and then runs a first quiet pull
(timeout 180 git pull -q) with a non-quiet retry, so update the logic to: detect
when DESTINATION exists but is not a directory (test -e "${DESTINATION}" && !
test -d "${DESTINATION}") and either remove or surface a clear error before
attempting git operations (same behavior as the git-clone-related block), and
make the retry pull use the same -q flag as the initial pull (apply -q to both
timeout 180 git pull invocations in the (timeout 180 git pull -q || (sleep 1m &&
(timeout 180 git pull -q || true))) construct) while keeping the timeout and
retry sleep behavior intact.

In @tests/git-clone-related-test/test-git-clone-related.sh:
- Around line 44-47: The "Done" log omits the $DEBUG variable, making it
inconsistent with the "About to run" message; update the final echo to include
$DEBUG (i.e., mirror the order used in the "About to run" line so it shows
${GIT_SCRIPTS}/git-clone-related $DEBUG $ARGS $resultdir), referencing the same
variables startdir, GIT_SCRIPTS, DEBUG, ARGS and resultdir so logs remain
comparable when debugging is enabled.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2189306 and 3c5cad5.

📒 Files selected for processing (3)
  • git-clone-or-pull
  • git-clone-related
  • tests/git-clone-related-test/test-git-clone-related.sh
🔇 Additional comments (7)
git-clone-related (1)

178-186: LGTM! Clean refactor of the retry logic.

The explicit if-based control flow is clearer than the previous brace/pipeline approach. The 180-second timeout and retry pattern appropriately handles network issues for large repositories.

tests/git-clone-related-test/test-git-clone-related.sh (1)

18-20: LGTM!

Clean approach for optional debug flag injection into the test harness.

git-clone-or-pull (5)

1-19: LGTM!

Clear documentation and proper argument validation.


21-41: LGTM!

Argument parsing correctly distinguishes between destination directory and git clone flags using the case statement pattern.


43-44: LGTM!

URL construction is straightforward. The ${DESTINATION_PARENT:?} is defensive, though effectively always set.


50-57: LGTM!

The clone retry pattern correctly mirrors the refactored git-clone-related logic with appropriate cleanup before retry.


60-60: LGTM!

Final output provides confirmation of the cloned/pulled commit.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @git-clone-or-pull:
- Around line 47-48: The pull branch currently swallows failures with "|| true"
and uses a 1-minute retry gap; remove "|| true" so a failing git pull returns a
non-zero exit code (propagating errors like the clone path), and change the
retry sleep from "sleep 1m" to "sleep 180s" to match the clone retry timing;
ensure the block around DESTINATION uses the existing timeout/git pull attempts
and exits non-zero if both attempts fail.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c5cad5 and 3c8efb8.

📒 Files selected for processing (2)
  • git-clone-or-pull
  • tests/git-clone-related-test/test-git-clone-related.sh
🔇 Additional comments (7)
git-clone-or-pull (5)

14-19: LGTM!

Argument validation is correct—requires at least ORG and REPO_NAME, provides helpful usage message to stderr, and exits with appropriate error code.


21-41: LGTM!

The beginswith helper is a clean POSIX-compatible approach. Argument parsing correctly differentiates between the optional destination parent and git clone arguments, and the default shallow-clone options are sensible.


43-44: LGTM!

URL construction is correct, and the ${DESTINATION_PARENT:?} expansion provides a defensive guard.


49-58: LGTM!

Clone logic correctly handles failures with cleanup of partial clones before retry, and properly exits with an error message on final failure.


60-60: LGTM!

Clear status output showing the repository's current HEAD revision.

tests/git-clone-related-test/test-git-clone-related.sh (2)

18-19: LGTM!

Clean approach for an optional debug flag with the commented example showing how to enable it.


44-47: LGTM!

The shellcheck disable is appropriate for intentional word splitting. The invocation correctly threads the DEBUG flag before ARGS.

Minor cosmetic note: when DEBUG is empty, the echo will show an extra space in the command display, but this doesn't affect functionality.

@mernst mernst merged commit 7df24a2 into main Jan 13, 2026
5 checks passed
@mernst mernst deleted the git-clone-or-pull branch January 13, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants